Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(locale): add Senegal locale #2525

Merged
merged 7 commits into from
Nov 7, 2023
Merged

feat(locale): add Senegal locale #2525

merged 7 commits into from
Nov 7, 2023

Conversation

makhtar-sarr
Copy link
Contributor

Description

This pull request adds a new locale for Senegal to Faker-js. It allows Faker-js users to generate Senegal-specific data, including names, addresses and phone numbers.

Changes made

  1. Creation of a local Senegal folder (fr_SN) in the appropriate directory.
  2. Set up Senegal-specific data, including names, addresses and phone numbers.
  3. Run pnpm run preflight.

History

Senegal is a West African country with its own culture and standards. Adding this locale will enable Faker-js users to generate data more relevant to Senegal, which can be useful for testing, generating demo data, etc.

Expected result

Faker-js users will be able to select the "senegal" locale and generate data that conforms to Senegalese culture and standards using Faker-js functions.

I will be available to answer your questions or make additional modifications if necessary.

@makhtar-sarr makhtar-sarr requested a review from a team November 3, 2023 20:21
@makhtar-sarr makhtar-sarr requested a review from a team as a code owner November 3, 2023 20:21
@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: locale Permutes locale definitions labels Nov 3, 2023
@ST-DDT ST-DDT added this to the vAnytime milestone Nov 3, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Nov 3, 2023

Hi @makhtar-sarr, thanks for your contribution.
Could you please do us a favor and split this PR into multiple smaller ones?
We prefer to review them at 1 PR per module (e.g. person/location), as checking foreign languages tends to take quite a while.
That way the first data can be merged without waiting on others. Please also include a description with wiki links or others sources for us to verify the entries (such as phone number patterns).
The first PR may contain both the metadata and the first module/locale category data (as well as the generated files).

See also:

@makhtar-sarr
Copy link
Contributor Author

Hello @ST-DDT,

Thank you for your feedback and constructive comments. I divided the PR into several small PR, in this first PR are the metadata and the internet module. As for the wiki links, I preferred to put them in this comment. Otherwise, if they're to appear as comments in the modules, I can do so.

Wiki links:
metadata

Thanks again for your responsiveness and your advice.

matthewmayer
matthewmayer previously approved these changes Nov 4, 2023
View reviewed changes
@matthewmayer
Copy link
Contributor

The failing tests are a known issue with the fr locale (see #2503 ) - to work around this, you need to add fr_SN to lines 46, 47 and 48 of test/all-functional.spec.ts

import-brain
import-brain previously approved these changes Nov 5, 2023
View reviewed changes
@ST-DDT
Copy link
Member

ST-DDT commented Nov 5, 2023

If you need help with the test fixes please let us know.

@makhtar-sarr
Copy link
Contributor Author

I modified lines 46, 47, and 48 of test/all-functional.spec.ts by adding fr_SN.

@codecov Codecov
Copy link

codecov bot commented Nov 5, 2023 •
edited
Loading

Codecov Report

Merging #2525 (a3b78b5) into next (b40ad45) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #2525      +/-   ##
==========================================
- Coverage   99.58%   99.57%   -0.01%     
==========================================
  Files        2781     2787       +6     
  Lines      249317   249378      +61     
  Branches     1084     1082       -2     
==========================================
+ Hits       248272   248328      +56     
- Misses       1017     1022       +5     
  Partials       28       28
Files Coverage Δ
src/locale/fr_SN.ts 100.00% <100.00%> (ø)
src/locale/index.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/index.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/internet/domain_suffix.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/internet/free_email.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/internet/index.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/metadata.ts 100.00% <100.00%> (ø)
src/locales/index.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@ST-DDT ST-DDT enabled auto-merge (squash) November 7, 2023 20:52
@ST-DDT ST-DDT merged commit 6df70bc into faker-js:next Nov 7, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@matthewmayer matthewmayer matthewmayer approved these changes

@import-brain import-brain Awaiting requested review from import-brain

Assignees

@makhtar-sarr makhtar-sarr

Labels
c: locale Permutes locale definitions p: 1-normal Nothing urgent
Projects
None yet
Milestone
vAnytime
Development

Successfully merging this pull request may close these issues.
4 participants
@makhtar-sarr @ST-DDT @matthewmayer @import-brain